Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide new Component type that represents the shape of components #11775

Merged
merged 3 commits into from
May 28, 2024

Conversation

dummdidumm
Copy link
Member

@dummdidumm dummdidumm commented May 24, 2024

In Svelte 3 and 4, components were classes under the hood, and the base class was SvelteComponent. This class was also used in language tools to properly type check the template code. In Svelte 5, components are functions. To give people a way to extend them programmatically, it would be good to expose the actual shape of components. This is why this PR introduces a new Component type.

For backwards compatibility reasons, we can't just get rid of the old class-based types. We also need to ensure that language tools can work with both the new and old types: There are many libraries out there that provide d.ts files with type definitions written using the class types - these should not error.

That's why there's an accompagning language tools PR (sveltejs/language-tools#2380) that's doing the heavy lifting: Instead of generating classes, it now generates a constant and an interfaces and uses Typescript's declaration merging feature to provide both so we can declare a component export as being both a class and a function. That ensures that people can still instantiate them with new (which they can do if they use the legacy.componentApi compiler option), and it also ensure we don't need to adjust any other code generation mechanisms in language tools yet - from a language tools perspective, classes are still the norm. But through exposing the default export as being also callable as a function we can in a future Svelte version, where classes/the Svelte 4 syntax are removed completely, seamlessly switch over to using functions in the code generation, too, and the d.ts files generated up until that point will support it because of the dual shape. This way we have both backwards and forwards compatibility.

Closes #11472

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with feat:, fix:, chore:, or docs:.
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests and linting

  • Run the tests with pnpm test and lint the project with pnpm lint

Copy link

changeset-bot bot commented May 24, 2024

🦋 Changeset detected

Latest commit: 984b5c5

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

…nents

In Svelte 3 and 4, components were classes under the hood, and the base class was `SvelteComponent`. This class was also used in language tools to properly type check the template code.
In Svelte 5, components are functions. To give people a way to extend them programmatically, it would be good to expose the actual shape of components. This is why this PR introduces a new `Component` type.
For backwards compatibility reasons, we can't just get rid of the old class-based types. We also need to ensure that language tools can work with both the new and old types: There are many libraries out there that provide `d.ts` files with type definitions written using the class types - these should not error.
That's why there's an accompagning language tools PR (sveltejs/language-tools#2380) that's doing the heavy lifting: Instead of generating classes, it now generates a constant and an interfaces and uses Typescript's declaration merging feature to provide both so we can declare a component export as being both a class and a function. That ensures that people can still instantiate them with `new` (which they can do if they use the `legacy.componentApi` compiler option), and it also ensure we don't need to adjust any other code generation mechanisms in language tools yet - from a language tools perspective, classes are still the norm. But through exposing the default export as being _also_ callable as a function we can in a future Svelte version, where classes/the Svelte 4 syntax are removed completely, seamlessly switch over to using functions in the code generation, too, and the `d.ts` files generated up until that point will support it because of the dual shape. This way we have both backwards and forwards compatibility.
packages/svelte/src/index.d.ts Outdated Show resolved Hide resolved
Comment on lines +157 to +158
/** Does not exist at runtime, for typing capabilities only. DO NOT USE */
z_$$bindings?: Bindings;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use @deprecated to make sure this sinks to the bottom, rather than the z_ prefix?

Suggested change
/** Does not exist at runtime, for typing capabilities only. DO NOT USE */
z_$$bindings?: Bindings;
/** @deprecated Does not exist at runtime, for typing capabilities only. DO NOT USE */
$$bindings?: Bindings;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer we don't because using it would print out "this is deprecated" warnings from svelte-check from the generated locations, and filtering those out is a bit tough. I'm also not sure if all IDEs move it to the bottom. Lastly, this is a very rare thing to show up anyway because it's on the function object, not the instance type.

@memestageceo
Copy link

How to use this? Shows 'svelte' has no exported member named Component.

this was working until I updated:

import type { ComponentType } from 'svelte';
export type IconType = {
	[key: string]: ComponentType<Icon>;
};

but replacing ComponentType with Component doesn't work.

dummdidumm added a commit to sveltejs/language-tools that referenced this pull request May 29, 2024
Svelte 5 uses functions to define components under the hood. This should be represented in the types. We can't just switch to using functions though because d.ts files created from Svelte 4 libraries should still work, and those contain classes. So we need interop between functions and classes. The idea is therefore:

Svelte 5 creates a default export which is both a function and a class constructor
Various places are adjusted to support the new default exports

Also see sveltejs/svelte#11775
@shinokada
Copy link

shinokada commented May 29, 2024

I have a similar problem with @aadtyaraj01 with VSCode.

When I use the following example code:

<script lang="ts">
  import type { Component } from 'svelte'
  type ListType = {
    name: string;
    href?: string;
    icon?: Component;
  };
  
 interface Props {
    sidebarList: ListType[];
// more lines
  }

  let {
    sidebarList,
// more lines
  }: Props = $props();
	
</script>

{#each sidebarList as { name, icon, href }}
	<svelte:component this={icon} />
  <a href={href}>{name}</a>
{/each}

I get the following type waring under {icon} in the svelte:component:

Argument of type 'Component<{}, {}, ""> | undefined' is not assignable to parameter of type 'ConstructorOfATypedSvelteComponent | null | undefined'.
Type 'Component<{}, {}, "">' is not assignable to type 'ConstructorOfATypedSvelteComponent'.
Type 'Component<{}, {}, "">' provides no match for the signature 'new (args: { target: any; props?: any; }): ATypedSvelteComponent'.

Possible causes:

  • You use the instance type of a component where you should use the constructor type
  • Type definitions are missing for this Svelte Component. If you are using Svelte 3.31+, use SvelteComponentTyped to add a definition:
    import type { SvelteComponentTyped } from "svelte";
    class ComponentName extends SvelteComponentTyped<{propertyName: string;}> {}ts(2345)

@J4gQBqqR
Copy link

Type 'typeof Overview__SvelteComponent_' is not assignable to type 'Component<{}, {}, "">'.
  Type 'typeof Overview__SvelteComponent_' provides no match for the signature '(internal: unknown, props: {}): { $on?(type: string, callback: (e: any) => void): () => void; $set?(props: Partial<{}>): void; }'.ts(2322)
Classes.svelte.ts(278, 3): The expected type comes from property 'component' which is declared here on type '{ title: string; component: Component<{}, {}, "">; }'
(property) component: Component<{}, {}, "">

@dummdidumm
Copy link
Member Author

I just released the corresponding updates to the VS Code extension and svelte-check, please try again with their latest versions

@shinokada
Copy link

@dummdidumm Thanks for a quick fix. My problem is solved.

@Dudek-AMS
Copy link
Contributor

Dudek-AMS commented Jun 1, 2024

I still have this issue in intellij

Svelte: Argument of type typeof Splashscreen__SvelteComponent_ is not assignable to parameter of type Component<{}, {}, "">
Type typeof Splashscreen__SvelteComponent_ provides no match for the signature
(internal: unknown, props: {}): { $on?(type: string, callback: (e: any) => void): () => void; $set?(props: Partial<{}>): void; }

//EDIT
i was able to fix it by manually update svelte-language-server in my npm modules and change it in the intellij settings to use this

@shinokada
Copy link

shinokada commented Jun 1, 2024

Hovering over the Component shows the following:

(alias) interface Component<Props extends Record<string, any> = {}, Exports extends Record<string, 
any> = {}, Bindings extends "" | keyof Props = "">
import Component
Can be used to create strongly typed Svelte components.

Example:
You have component library on npm called component-library, from which you export a component called 
MyComponent. For Svelte+TypeScript users, you want to provide typings. Therefore you create a index.d.ts:

import { Component } from "svelte";
export declare const MyComponent: Component<{ foo: string }> {}
Typing this makes it possible for IDEs like VS Code with the Svelte extension to provide intellisense and 
to use the component like this in a Svelte file with TypeScript:

<script lang="ts">
    import { MyComponent } from "component-library";
</script>
<MyComponent foo={'bar'} />

Currently when I run npm run package, it create dist/index.d.ts that is the same as index.js.

export { default as MyComponent } from './MyComponent.svelte'

Does this mean library creators need to add the above lines to dist/index.d.ts?

@dummdidumm
Copy link
Member Author

How does your index.js look like?

@shinokada
Copy link

For example:

export { default as GitHub } from './GitHub.svelte'
export { default as GitHubDownloads } from './GitHubDownloads.svelte'
export { default as GitHubSponsors } from './GitHubSponsors.svelte'
export { default as JsrVersion } from './JsrVersion.svelte'
export { default as License } from './License.svelte'
// more lines

@dummdidumm
Copy link
Member Author

No you don't need to do anything when using svelte-package. We should add that to the hover info

@MerlinB
Copy link

MerlinB commented Jun 3, 2024

When I try to pass a component as prop using the latest versions of svelte and svelte-check, I now get problems with the binding parameter:

Type '__sveltets_2_IsomorphicComponent<{ [x: string]: never; }, { [evt: string]: CustomEvent<any>; }, {}, Record<string, any>, string>' is not assignable to type 'Component<{}, {}, "">'.
  Types of property 'z_$$bindings' are incompatible.
    Type 'string | undefined' is not assignable to type '"" | undefined'.

Example:

<script lang="ts">
	import Icon from "./Icon.svelte"
	import IconWrapper from "./IconWrapper.svelte"
</script>

<IconWrapper icon={Icon} />

IconWrapper.svelte

<script lang="ts">
	import type { Component } from "svelte"
	 
	let {icon}: {
		icon: Component
	} = $props()
</script>

<svelte:component this={icon} />

@Dudek-AMS
Copy link
Contributor

can confirm, same issue

<script lang="ts">
    import SceneStore from '$lib/SceneStore.svelte';

    import Logo from '$assets/Logo_MagicTale_Mobile.png';
    import Login, {load} from "./Login.svelte";


    $effect(() => {
        setTimeout(() => {
            SceneStore.changeScene(Login, load)
        }, 2000)
    })

</script>

while Login does not use $props()

image

@J4gQBqqR
Copy link

J4gQBqqR commented Jun 4, 2024

Exactly what I saw, I have my imported component typed as:

(alias) const Overview: __sveltets_2_IsomorphicComponent<Record<string, never>, {
    [evt: string]: CustomEvent<any>;
}, {}, Record<string, any>, string>
export Overview

Therefore VS code gives me:

Type '__sveltets_2_IsomorphicComponent<Record<string, never>, { [evt: string]: CustomEvent<any>; }, {}, Record<string, any>, string>' is not assignable to type 'Component<{}, {}, "">'.
  Types of property 'z_$$bindings' are incompatible.
    Type 'string | undefined' is not assignable to type '"" | undefined'.
      Type 'string' is not assignable to type '""'.ts(2322)

@J4gQBqqR
Copy link

J4gQBqqR commented Jun 4, 2024

Not sure how __sveltets_2_IsomorphicComponent is generated automatically while I import my component.

dummdidumm added a commit that referenced this pull request Jun 6, 2024
The current type narrows the binding type to `""` by default, which means "no bindings on this component". While this is the common case, it makes it very cumbersome to use the `Component` type because legacy components are of type `string` and as soon as you have bindings, the type is something like `"foo" | "bar"` which _also_ is not assignable to `""` which is semantically wrong, because you should be able to assign a component that can have bindings to a type that accepts none.
The pragmatic solution is to change the binding type to allow `string`, which means someone theoretically could use bindings with a component that doesn't have bindings:
```svelte
<script>
  let component: Component<{ prop: boolean }> = IAcceptNoBindings;
</script>
<!-- allowed but should be a type error -->
<svelte:component this={component} bind:prop={foo} />
```
But this is a) rare anyway and b) can be caught at runtime

This came up in comments of #11775
@dummdidumm dummdidumm mentioned this pull request Jun 6, 2024
5 tasks
Rich-Harris pushed a commit that referenced this pull request Jun 6, 2024
The current type narrows the binding type to `""` by default, which means "no bindings on this component". While this is the common case, it makes it very cumbersome to use the `Component` type because legacy components are of type `string` and as soon as you have bindings, the type is something like `"foo" | "bar"` which _also_ is not assignable to `""` which is semantically wrong, because you should be able to assign a component that can have bindings to a type that accepts none.
The pragmatic solution is to change the binding type to allow `string`, which means someone theoretically could use bindings with a component that doesn't have bindings:
```svelte
<script>
  let component: Component<{ prop: boolean }> = IAcceptNoBindings;
</script>
<!-- allowed but should be a type error -->
<svelte:component this={component} bind:prop={foo} />
```
But this is a) rare anyway and b) can be caught at runtime

This came up in comments of #11775
@xeho91
Copy link
Contributor

xeho91 commented Jun 17, 2024

I've got a question related to this new type feature.

import type { Component, SvelteComponent } from "svelte";

import Button from "./Button.svelte"; // Svelte v5 component

type IsComponent = Button extends Component ? true : false;
//.  ^ false - ❌ not expected

type IsComponentWithTypeOf = typeof Button extends Component ? true : false;
//.  ^ true - ✅ expected

type IsSvelteComponent = Button extends SvelteComponent ? true : false;
//.  ^ true - ❌ not expected

type IsSvelteComponentWithTypeOf = typeof Button extends SvelteComponent ? true : false;
//.  ^ false - ✅ expected

I think the latest version of svelte-language-server treats every *.svelte import by default as SvelteComponent - I can confirm it by hovering on the definition of Button import specifier. See screenshot:

image

Source code of Button.svelte
<script lang="ts">
  import type { Snippet } from 'svelte';
  import type { HTMLButtonAttributes } from 'svelte/elements';

  interface Props extends HTMLButtonAttributes {
    /**
     * Is this the principal call to action on the page?
     */
    primary?: boolean;
    /**
     * What background color to use
     */
    backgroundColor?: string;
    /**
     * How large should the button be?
     */
    size?: 'small' | 'medium' | 'large';

    /**
     * Content of the button
     */
    children?: Snippet;
  }

  const {
    primary = true,
    backgroundColor,
    size = 'medium',
    children,
    ...buttonProps
  }: Props = $props();
</script>

<button
  type="button"
  class:primary
  class={size}
  style={backgroundColor ? `background-color: ${backgroundColor}` : ''}
  {...buttonProps}
>
  {#if children}
    {@render children()}
  {/if}
</button>

<style>
  button {
    font-family: 'Nunito Sans', 'Helvetica Neue', Helvetica, Arial, sans-serif;
    font-weight: 700;
    border: 0;
    border-radius: 3em;
    cursor: pointer;
    display: inline-block;
    line-height: 1;

    color: #333;
    background-color: transparent;
    box-shadow: rgba(0, 0, 0, 0.15) 0px 0px 0px 1px inset;
  }

  .primary {
    color: white;
    background-color: #1ea7fd;
    box-shadow: unset;
  }

  .small {
    font-size: 12px;
    padding: 10px 16px;
  }

  .medium {
    font-size: 14px;
    padding: 11px 20px;
  }
  .large {
    font-size: 16px;
    padding: 12px 24px;
  }
</style>

Question

Is there anything I can do to not use typeof? in order to correctly infer the type of Button Svelte component?

What I am trying to achieve is to create a type helper Meta<TComponent extends Component> for a library - Storybook addon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Svelte 5: ComponentType is still a constructor
8 participants